Scheduler - Appointments Refactoring - Custom Templates#33159
Scheduler - Appointments Refactoring - Custom Templates#33159Tucchhaa wants to merge 2 commits intoDevExpress:25_2from
Conversation
| } | ||
|
|
||
| viewModelDiff.forEach((diffItem) => { | ||
| viewModelDiff.forEach((diffItem, index) => { |
There was a problem hiding this comment.
Passing index from diff may seem incorrect, but it is exactlye how the old implementation works:
To avoid making BC I have passed index too, but in general it seems that we need to remove passing index to appointmentTemplate function
There was a problem hiding this comment.
How about adding a todo to remove index in future? Seems valid in this case
aleksei-semikozov
left a comment
aleksei-semikozov
left a comment
There was a problem hiding this comment.
option('appointmentTemplate', ...) silent no-op
m_scheduler.ts _optionChanged does this._appointments.option('itemTemplate', value). new Appointments has no itemTemplate — only appointmentTemplate. so under _newAppointments: true, runtime template change does nothing. no test catches it (every test sets the template at construction).
new Appointments._optionChanged ignores templates
appointments.ts:88-109 only handles items and viewModel. appointmentTemplate, appointmentCollectorTemplate, currentView, onAppointmentRendered, $allDayContainer — all no-op.
stale TODO
m_scheduler.ts:1055 // TODO<Appointments>: set custom templates. this PR closes it. drop.
naming appointments_new
ages badly. you wrote in issue it becomes appointments/ after old removed. fine temporarily. while it lives the name says nothing. _lite or _pure would say "no CollectionWidget bloat". minor.
| // @ts-expect-error | ||
| $scheduler.dxScheduler('dispose'); | ||
| document.body.innerHTML = ''; | ||
| jest.useRealTimers(); |
There was a problem hiding this comment.
dead. no fake timers in file. drop.
| expect($appointment.text()).toBe('Appointment 1 ViewTemplate'); | ||
| }); | ||
|
|
||
| it('should render async template', async () => { |
There was a problem hiding this comment.
only happy path. add:
dispose()during pending promise → no DOM write after disposeoption('appointmentTemplate', x)before previous resolves
base_appointment.ts:127 when().done() has no stale guard.
| @@ -174,13 +174,24 @@ describe('AppointmentCollector', () => { | |||
| { isCompact: false, expectedText: '1 more' }, | |||
| ])('should have correct text for appointmentsCount = 1 and isCompact = %o', ({ isCompact, expectedText }) => { | |||
There was a problem hiding this comment.
appointmentsCount gone, replaced by appointmentsData. rename describe to e.g. for items.length === 1.
| } | ||
|
|
||
| viewModelDiff.forEach((diffItem) => { | ||
| viewModelDiff.forEach((diffItem, index) => { |
There was a problem hiding this comment.
agree with what you wrote in your own comment below — the diff index isn't a stable visible index, this is BC parity with a legacy bug. let's track it as a follow-up so it doesn't get forgotten and fix it the way you suggested in #3612 (drop passing the arg).
3 participants
No description provided.